Skip to content

Support for editing config file (to an extent) via web dashboard#49

Merged
luke-jr merged 15 commits intoOCEAN-xyz:masterfrom
luke-jr:confrw
Mar 10, 2025
Merged

Support for editing config file (to an extent) via web dashboard#49
luke-jr merged 15 commits intoOCEAN-xyz:masterfrom
luke-jr:confrw

Conversation

@luke-jr
Copy link
Copy Markdown
Contributor

@luke-jr luke-jr commented Dec 13, 2024

Built on top of #25

Mostly functional, two FIXMEs remain that should have minimal issues until followup. Would be nice to avoid restarting ever, but that can be a future improvement.

@luke-jr luke-jr added the enhancement New feature or request label Dec 13, 2024
@luke-jr luke-jr requested a review from wizkid057 December 13, 2024 23:02
@luke-jr luke-jr added this to the v0.3.0 milestone Dec 14, 2024
@luke-jr luke-jr mentioned this pull request Dec 14, 2024
Copy link
Copy Markdown
Member

@wizkid057 wizkid057 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK

Copy link
Copy Markdown
Member

@wizkid057 wizkid057 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3bb50b8

Sorry, missed this when reviewing... GBT cURL call has an aggressive timeout so that the block template thread never hangs.

Booting all stratum clients after a single failure is probably too aggressive, considering the 1 second retry already in play. Any number of benign things can prevent this call from returning, especially if the node isn't on the same machine.

If we are working on something valid, I'd suggest not booting stratum clients until GBT fails for long enough for the the current job to be stale.

@szarka
Copy link
Copy Markdown
Contributor

szarka commented Dec 16, 2024

Is there anything in this patch, or elsewhere, that adds access control for the dashboard? AFAIK, that has to be done external to datum_gateway itself right now, but it's not too much of a threat to just display the stats. Adding config editing changes that.

@luke-jr
Copy link
Copy Markdown
Contributor Author

luke-jr commented Dec 17, 2024

@szarka This feature is disabled by default and needs to be turned on with a new config file setting.

Currently, it does not have any special access control, and that could be a possible follow-up PR.

Note that it intentionally does not "read back" the RPC password, only allows changing it.

@szarka
Copy link
Copy Markdown
Contributor

szarka commented Dec 17, 2024

@szarka This feature is disabled by default and needs to be turned on with a new config file setting.

Nice. 👍

@BitcoinMechanic
Copy link
Copy Markdown
Contributor

Let's add authentication to this. Should require (or least give the user the option of requiring) username/password to log in to the dashboard.

@luke-jr
Copy link
Copy Markdown
Contributor Author

luke-jr commented Jan 15, 2025

Not sure authentication is realistic: right now, the webserver is entirely unencrypted and can just be MITM'd...

@luke-jr luke-jr force-pushed the confrw branch 2 times, most recently from 882c6db to 2201df6 Compare January 16, 2025 16:19
@luke-jr luke-jr marked this pull request as draft January 16, 2025 17:25
@luke-jr luke-jr marked this pull request as ready for review March 8, 2025 06:12
@luke-jr
Copy link
Copy Markdown
Contributor Author

luke-jr commented Mar 8, 2025

Rebased

  • Now requires authentication (encryption left as an exercise for a wrapper)
  • Updates new bitcoind_rpcuserpass cache when user/password change

Copy link
Copy Markdown
Member

@wizkid057 wizkid057 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK

Some notes:
Pooled vs Non-Pooled is a little confusing here in the web configurator vs the config file help doc. The web configurator always puts the default pool host in the form field, regardless of pooled/non-pooled, but doesn't actually save it to the DATUM pool host if non-pooled is selected... which isn't exactly what I'd expect from that. Maybe if non-pooled is selected, the pool host form field should be disabled, when possible (probably via JS).

We may want to refactor this someday to better use info from datum_conf a bit more automagically vs the heavily manual list of variables that can be used with this. Including sufficient data in the configuration item structure to accomplish validation and whatnot, along with human descriptions, etc might be the way to go for this one day.

@luke-jr luke-jr merged commit 4437536 into OCEAN-xyz:master Mar 10, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants